Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure a dependency exists when checking for errors and validity #434

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

znewsham
Copy link

@znewsham znewsham commented Apr 9, 2021

Fixes #359

  1. add a new "private" function _ensureKeyDep that will add a dependency if Tracker is provided to the schema and no dependency exists.
  2. call _ensureKeyDep from keyIsInvalid and keyErrorMessage

All tests are passing

@znewsham znewsham changed the title En Ensure a dependency exists when checking for errors and validity Apr 9, 2021
@aldeed
Copy link
Collaborator

aldeed commented Apr 14, 2021

I don't think the Meteor tests run on GitHub, and since this is a Meteor thing, I'll have to confirm this in the Meteor app. It should be possible to add tests for it in tests/meteor-app, too. My only concern is needing to look at the way that deps are marked changed, because it doesn't help to have the dep if a different dep is marked as changed. In particular, if I change a label or validate at the subschema level, is it reactive for the parent schema? Should it be?

It seems like the most elegant solution would be to have validation bubble up and down so that every schema involved is aware of what the current state is, and is reactive to it.

@znewsham
Copy link
Author

I'm really nervous about bubbling - autoform (where I mostly use schema + tracker) is already a little over-reactive.

The change in this PR ensures that whatever the "base" schema is (in use by autoform) will track all dependencies within that form - as validation is performed on the validation context attached to the base schema.

If the concern is that field a is an object, and field a.b has a validation error, but you want to check to see if field a is valid - I've resolved this issue by adding a new method to my local version of ValidationContext that you can ask specifically if "a or any of it's children are invalid" - this way, in the handful of cases that I need this type of (expensive) reactivity, I have access to it - but the 95% of the places I dont need this, dont need to pay the price.

I'll look into adding meteor tests at some point when I have time, but tbh, I've pretty much made the decision to maintain my own local version of simpl-schema now, as a lot of the changes I want, need to be made to the core simpl-schema code, but don't necessarily make sense to everyone else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Validation Errors not showing for sub schemas
2 participants